Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Being more defensive when finding a container for a nav item #31

Merged
merged 1 commit into from
Jun 26, 2018

Conversation

dapenguin
Copy link
Contributor

@dapenguin dapenguin commented Jun 26, 2018

When a nav container wasn't found, we were getting errors that prevented the rest of the JavaScript from running.

Have yet to figure out why this has all of a sudden become a problem though.

@dapenguin dapenguin requested a review from thurstontye June 26, 2018 10:10
Copy link

@thurstontye thurstontye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
NB: Could use o-header-services__primary-nav but I understand this correctly it's to insert the republish nav item. Not sure why we would want ever want to do that on republishing

@dapenguin
Copy link
Contributor Author

@Thurst-ft Agree that we don't really need this on Republishing. I've raised issue #32 to sort this out.

@dapenguin dapenguin merged commit 7340f5d into master Jun 26, 2018
@dapenguin dapenguin deleted the fix-nav-js-error branch July 23, 2018 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants